Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: use test_interactive_test_util when possible #12461

Merged
merged 6 commits into from
Nov 27, 2019

Conversation

fjmolinas
Copy link
Contributor

Contribution description

In order to be able to run tests on as many boards as possible use test_interactive_test_util in all possible cases.

Many tests depend on resetting the board after the terminal has been opened to make sure the captured output is the start of the tests.

When BOARD's can't provide this feature using test_interactive_test_util allows the application to wait for a sync from the test script to actually start.

Changes introduced:

  • 0e1c79e: modify test runner to accept a sync option
  • 28948d6: add shell commands for interactive sync
  • faf8fa1: used interactive sync when possible. The module is added in auto-init. I did't split the commit since introducing it before the test changes would make most tests failed.

Can be splited from the PR:

  • e5b1cd7: improve tests/isr_yield_higher

TODO in follow-up:

  • reset before term
  • not reset if sync=True:
    if sync is True:
        try:
            subprocess.check_output(('make', 'reset'), env=env,
                                    stderr=subprocess.PIPE)
        except subprocess.CalledProcessError:
            # make reset yields error on some boards even if successful
            pass

Testing procedure

  • Run all tests on multiple boards that support resetting, nothing should have changed:
native
python3 dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . native  --with-test-only
ERROR:native:Tests failed: 7
Failures during test:
- [tests/gnrc_ipv6_ext](tests/gnrc_ipv6_ext/test.failed)
- [tests/gnrc_ipv6_ext_frag](tests/gnrc_ipv6_ext_frag/test.failed)
- [tests/gnrc_rpl_srh](tests/gnrc_rpl_srh/test.failed)
- [tests/gnrc_sock_dns](tests/gnrc_sock_dns/test.failed)
- [tests/gnrc_tcp](tests/gnrc_tcp/test.failed)
- [tests/lwip](tests/lwip/test.failed)
- [tests/netstats_l2](tests/netstats_l2/test.failed)
samr21-xpro
python3 dist/tools/compile_and_test_for_board/compile_and_test_for_board.py . samr21-xpro  --with-test-only
Failures during test:
- [examples/suit_update](examples/suit_update/test.failed)
- [tests/gnrc_ipv6_ext](tests/gnrc_ipv6_ext/test.failed)
- [tests/gnrc_ipv6_ext_frag](tests/gnrc_ipv6_ext_frag/test.failed)
- [tests/gnrc_rpl_srh](tests/gnrc_rpl_srh/test.failed)
- [tests/gnrc_sock_dns](tests/gnrc_sock_dns/test.failed)
- [tests/gnrc_tcp](tests/gnrc_tcp/test.failed)
- [tests/pkg_fatfs_vfs](tests/pkg_fatfs_vfs/test.failed)

iotlab-m3
ERROR:iotlab-m3:Tests failed: 7
Failures during test:
- [examples/suit_update](examples/suit_update/test.failed)
- [tests/gnrc_ipv6_ext](tests/gnrc_ipv6_ext/test.failed)
- [tests/gnrc_ipv6_ext_frag](tests/gnrc_ipv6_ext_frag/test.failed)
- [tests/gnrc_rpl_srh](tests/gnrc_rpl_srh/test.failed)
- [tests/gnrc_sock_dns](tests/gnrc_sock_dns/test.failed)
- [tests/gnrc_tcp](tests/gnrc_tcp/test.failed)
- [tests/pkg_fatfs_vfs](tests/pkg_fatfs_vfs/test.failed)


  • Green murdock to see I haven't missed any dependency

Issues/PRs references

Part of #12448

@fjmolinas fjmolinas added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: tests Area: tests and testing framework CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Oct 15, 2019
@fjmolinas
Copy link
Contributor Author

fjmolinas commented Oct 16, 2019

With this PR test failed goes down from to 16 for ``z1`

PR:ERROR:z1:Tests failed: 76
Failures during test:
- [tests/bench_runtime_coreapis](tests/bench_runtime_coreapis/test.failed)
- [tests/bench_sizeof_coretypes](tests/bench_sizeof_coretypes/test.failed)
- [tests/bitarithm_timings](tests/bitarithm_timings/test.failed)
- [tests/build_system_cflags_spaces](tests/build_system_cflags_spaces/test.failed)
- [tests/buttons](tests/buttons/test.failed)
- [tests/cb_mux](tests/cb_mux/test.failed)
- [tests/cb_mux_bench](tests/cb_mux_bench/test.failed)
- [tests/driver_grove_ledbar](tests/driver_grove_ledbar/test.failed)
- [tests/driver_my9221](tests/driver_my9221/test.failed)
- [tests/embunit](tests/embunit/test.failed)
- [tests/event_wait_timeout](tests/event_wait_timeout/test.failed)
- [tests/evtimer_msg](tests/evtimer_msg/test.failed)
- [tests/float](tests/float/test.failed)
- [tests/fmt_print](tests/fmt_print/test.failed)
- [tests/gnrc_ipv6_nib](tests/gnrc_ipv6_nib/test.failed)
- [tests/gnrc_ipv6_nib_6ln](tests/gnrc_ipv6_nib_6ln/test.failed)
- [tests/gnrc_ndp](tests/gnrc_ndp/test.failed)
- [tests/gnrc_sock_ip](tests/gnrc_sock_ip/test.failed)
- [tests/gnrc_sock_udp](tests/gnrc_sock_udp/test.failed)
- [tests/heap_cmd](tests/heap_cmd/test.failed)
- [tests/irq](tests/irq/test.failed)
- [tests/isr_yield_higher](tests/isr_yield_higher/test.failed)
- [tests/l2util](tests/l2util/test.failed)
- [tests/libc_newlib](tests/libc_newlib/test.failed)
- [tests/log_color](tests/log_color/test.failed)
- [tests/log_printfnoformat](tests/log_printfnoformat/test.failed)
- [tests/msg_avail](tests/msg_avail/test.failed)
- [tests/msg_send_receive](tests/msg_send_receive/test.failed)
- [tests/msg_try_receive](tests/msg_try_receive/test.failed)
- [tests/mutex_order](tests/mutex_order/test.failed)
- [tests/netdev_test](tests/netdev_test/test.failed)
- [tests/od](tests/od/test.failed)
- [tests/periph_gpio](tests/periph_gpio/test.failed)
- [tests/periph_timer](tests/periph_timer/test.failed)
- [tests/pipe](tests/pipe/test.failed)
- [tests/pkg_c25519](tests/pkg_c25519/test.failed)
- [tests/pkg_cayenne-lpp](tests/pkg_cayenne-lpp/test.failed)
- [tests/pkg_heatshrink](tests/pkg_heatshrink/test.failed)
- [tests/pkg_jsmn](tests/pkg_jsmn/test.failed)
- [tests/pkg_lora-serialization](tests/pkg_lora-serialization/test.failed)
- [tests/pkg_nanocbor](tests/pkg_nanocbor/test.failed)
- [tests/pkg_tiny-asn1](tests/pkg_tiny-asn1/test.failed)
- [tests/pkg_tweetnacl](tests/pkg_tweetnacl/test.failed)
- [tests/pkg_ucglib](tests/pkg_ucglib/test.failed)
- [tests/pkg_umorse](tests/pkg_umorse/test.failed)
- [tests/posix_semaphore](tests/posix_semaphore/test.failed)
- [tests/pthread](tests/pthread/test.failed)
- [tests/pthread_barrier](tests/pthread_barrier/test.failed)
- [tests/pthread_cleanup](tests/pthread_cleanup/test.failed)
- [tests/pthread_condition_variable](tests/pthread_condition_variable/test.failed)
- [tests/pthread_cooperation](tests/pthread_cooperation/test.failed)
- [tests/pthread_flood](tests/pthread_flood/test.failed)
- [tests/pthread_rwlock](tests/pthread_rwlock/test.failed)
- [tests/pthread_tls](tests/pthread_tls/test.failed)
- [tests/riotboot_hdr](tests/riotboot_hdr/test.failed)
- [tests/rmutex](tests/rmutex/test.failed)
- [tests/sched_testing](tests/sched_testing/test.failed)
- [tests/shell](tests/shell/test.failed)
- [tests/test_tools](tests/test_tools/test.failed)
- [tests/thread_basic](tests/thread_basic/test.failed)
- [tests/thread_cooperation](tests/thread_cooperation/test.failed)
- [tests/thread_exit](tests/thread_exit/test.failed)
- [tests/thread_flags](tests/thread_flags/test.failed)
- [tests/thread_flags_xtimer](tests/thread_flags_xtimer/test.failed)
- [tests/thread_flood](tests/thread_flood/test.failed)
- [tests/thread_msg](tests/thread_msg/test.failed)
- [tests/thread_msg_block_w_queue](tests/thread_msg_block_w_queue/test.failed)
- [tests/thread_msg_block_wo_queue](tests/thread_msg_block_wo_queue/test.failed)
- [tests/thread_msg_seq](tests/thread_msg_seq/test.failed)
- [tests/thread_race](tests/thread_race/test.failed)
- [tests/trickle](tests/trickle/test.failed)
- [tests/xtimer_hang](tests/xtimer_hang/test.failed)
- [tests/xtimer_msg_receive_timeout](tests/xtimer_msg_receive_timeout/test.failed)
- [tests/xtimer_now64_continuity](tests/xtimer_now64_continuity/test.failed)
- [tests/xtimer_periodic_wakeup](tests/xtimer_periodic_wakeup/test.failed)
- [tests/xtimer_remove](tests/xtimer_remove/test.failed)
- [tests/xtimer_reset](tests/xtimer_reset/test.failed)
- [tests/xtimer_usleep](tests/xtimer_usleep/test.failed)

PR:ERROR:z1:Tests failed: 16
Failures during compilation:
- [tests/gnrc_ndp](tests/gnrc_ndp/compilation.failed)

Failures during test:
- [tests/bench_runtime_coreapis](tests/bench_runtime_coreapis/test.failed)
- [tests/bitarithm_timings](tests/bitarithm_timings/test.failed)
- [tests/driver_grove_ledbar](tests/driver_grove_ledbar/test.failed)
- [tests/driver_my9221](tests/driver_my9221/test.failed)
- [tests/event_wait_timeout](tests/event_wait_timeout/test.failed)
- [tests/evtimer_msg](tests/evtimer_msg/test.failed)
- [tests/periph_gpio](tests/periph_gpio/test.failed)
- [tests/pkg_c25519](tests/pkg_c25519/test.failed)
- [tests/pkg_heatshrink](tests/pkg_heatshrink/test.failed)
- [tests/pkg_lora-serialization](tests/pkg_lora-serialization/test.failed)
- [tests/pkg_tiny-asn1](tests/pkg_tiny-asn1/test.failed)
- [tests/pkg_tweetnacl](tests/pkg_tweetnacl/test.failed)
- [tests/pthread_tls](tests/pthread_tls/test.failed)
- [tests/shell](tests/shell/test.failed)
- [tests/thread_flags_xtimer](tests/thread_flags_xtimer/test.failed)

Te compilation issue in tests/gnrc_ndp is because of fw size.

@fjmolinas
Copy link
Contributor Author

I'm going to trigger murdock to see which applications don't compile because of size anymore.

@@ -22,10 +22,12 @@
TIMEOUT = int(os.environ.get('RIOT_TEST_TIMEOUT') or 10)


def run(testfunc, timeout=TIMEOUT, echo=True, traceback=False):
def run(testfunc, timeout=TIMEOUT, echo=True, traceback=False, sync=False):
Copy link
Contributor

@keestux keestux Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered to make sync default to True? Wouldn't that reduce the number of files you have to change for this PR?
There are 168 line with "run(testfunc" and in the PR 154 with sync=True.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I didn't initially because I wasn't sure if the PR would need to be split and introduce changes slowly, in which case a default false was better.

I can remove them at the end, for now I rather keep it help me see easily which tests I have changed and which not :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove them at the end, for now I rather keep it help me see easily which tests I have changed and which not :)

By the end I mean after squashing :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is time now, if all tests are not regressing, please squash and update the with default sync=True

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll test the squash version then push.

tests/README.md Outdated Show resolved Hide resolved
@fjmolinas fjmolinas added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Oct 22, 2019
@fjmolinas fjmolinas force-pushed the pr_apply_interactive_sync_tests branch from fee1d9e to 3245eaa Compare October 22, 2019 09:14
@fjmolinas
Copy link
Contributor Author

rebased

@fjmolinas fjmolinas force-pushed the pr_apply_interactive_sync_tests branch from 3245eaa to ddce923 Compare October 22, 2019 14:55
@fjmolinas
Copy link
Contributor Author

rebased

@fjmolinas fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 22, 2019
@MrKevinWeiss
Copy link
Contributor

I can make this a priority if you have time to work on it (I think we need to move fast due to moving target rebasing).

My only concern with this is that it changes the behaviour of the tests when you just want to term in and read values. if there is a way to include everything only when make test is called and not when make term is.

Also I think it would be good to evaluate this on the arduino-mega2560. I have had some problems with getting stuck in the bootloader mode when trying to poll if the terminal is ready (I believe you need a 2.6 second timeout after opening a serial connection otherwise it could take up to 10 seconds or so to go out of bootloader mode, this 10 seconds resets whenever you try to talk to it). I don't see this problem on a cheap offbrand china version but on the standard ones this is how the bootloader seems to behave.

@fjmolinas
Copy link
Contributor Author

I can make this a priority if you have time to work on it (I think we need to move fast due to moving target rebasing).

I was on holiday but I'm back and can get back to working on this.

Also I think it would be good to evaluate this on the arduino-mega2560. I have had some problems with getting stuck in the bootloader mode when trying to poll if the terminal is ready (I believe you need a 2.6 second timeout after opening a serial connection otherwise it could take up to 10 seconds or so to go out of bootloader mode, this 10 seconds resets whenever you try to talk to it). I don't see this problem on a cheap offbrand china version but on the standard ones this is how the bootloader seems to behave.

I will test and see what happens.

My only concern with this is that it changes the behaviour of the tests when you just want to term in and read values. if there is a way to include everything only when make test is called and not when make term is.

I'll look into this

@fjmolinas
Copy link
Contributor Author

rebased

@fjmolinas
Copy link
Contributor Author

Failing tests:

    tests/driver_nrfmin/nrf52dk:gnu
    tests/driver_nrfmin/nrf52dk:llvm
    tests/malloc/nrf52dk:llvm
    tests/nordic_softdevice/nrf52dk:gnu

The first two and the 4th are fixed by removing the child.expect_exact("All up, running the shell now"). I can't reproduce the third failure locally.

@fjmolinas fjmolinas removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 26, 2019
@fjmolinas fjmolinas force-pushed the pr_apply_interactive_sync_tests branch from 9979fb5 to cf95904 Compare November 26, 2019 16:08
@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 26, 2019
@aabadie
Copy link
Contributor

aabadie commented Nov 26, 2019

@fjmolinas, please add arduino-uno, arduino-duemilanove, arduino-nano and atmega328p boards to BOARD_INSUFFICIENT_MEMORY and we are good to go with this one!

@fjmolinas
Copy link
Contributor Author

I think tests/periph_wdt migh have failed because of #12812 (comment).

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

Once the CI is green, this can be merged.

fjmolinas and others added 6 commits November 27, 2019 15:07
Automatically call the test_utils_interactive_sync synchronization if it
is used.
- Define test_utils_interactive_sync as DEFAULT_MODULE in Makefile.tests_common
- For tests disabling autoinit, add test_utils_interactive_sync to main
- Add DISABLE_MODULE += test_utils_interactive_sync for tests requiring
  sudo,  `tests/shell`, `tests/minimal` and `tests/stdin`
- Add shell_commands to tests/periph_wdt and tests/struct_tm_utility to
  pull `r` and `s` commands
- Remove includes and usage in `tests/main.c` for tests that where
  already using test_utils_interactive_sync
- When using test_interactive_sync_utils, stdin and many more
  prints/puts are included. These all go into .bss/.data which
  quickly fills up RAM.
@fjmolinas fjmolinas force-pushed the pr_apply_interactive_sync_tests branch from 513d4c5 to 175c48f Compare November 27, 2019 14:08
fjmolinas added a commit to fjmolinas/RIOT that referenced this pull request Nov 27, 2019
@aabadie
Copy link
Contributor

aabadie commented Nov 27, 2019

All green!

@aabadie aabadie merged commit 6e2513a into RIOT-OS:master Nov 27, 2019
@fjmolinas
Copy link
Contributor Author

@keestux @aabadie Thanks for the review! @MrKevinWeiss Thanks for testing!

@fjmolinas fjmolinas deleted the pr_apply_interactive_sync_tests branch November 27, 2019 18:28
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
fjmolinas added a commit to fjmolinas/RIOT that referenced this pull request Mar 24, 2020
With RIOT-OS#12941 and RIOT-OS#13613 some of the blacklisting introduced in RIOT-OS#12461
are no longer needed, since `test_interactive_test_util` is lighter
or adds no extra code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework CI: disable test cache If set, CI will always run all tests regardless of whether they have been run successfully before CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants